fix(type): remove makeObservable and makeAutoObservable function proxy option #3717
Conversation
🦋 Changeset detectedLatest commit: 722b126 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
liucan233
changed the title
|
Makes me wonder whether the setting shouldn't be part of annotation/enhancer, so it propagates to nested structures, but probably not worth the additional complexity. |
| // const foo = observable({}), foo.bar = 1 will be monitored, | ||
| // const foo = observable({}, {proxy: false}), foo.bar = 1 will be not monitored | ||
| // The summary is that proxy is an invalid option for makeObservable and makeAutoObservable | ||
| type NoProxyCreateObservableOptions = Omit<CreateObservableOptions, "proxy"> |
docs/observable-state.md
Outdated
| @@ -136,6 +136,20 @@ Inference rules: | |||
| - All _generator_ functions become `flow`. (Note that generator functions are not detectable in some transpiler configurations, if flow doesn't work as expected, make sure to specify `flow` explicitly.) | |||
| - Members marked with `false` in the `overrides` argument will not be annotated. For example, using it for read only fields such as identifiers. | |||
|
|
|||
| ## Options {🚀} | |||
There was a problem hiding this comment.
Remove the added section and change the original:
proxy: falseforcesobservable(thing)to use non-proxy implementation. This is a good option if the shape of the object will not change over time, as non-proxied objects are easier to debug and faster. See avoiding proxies.
=>
proxy: falseforcesobservable(thing)to use non-proxy implementation. This is a good option if the shape of the object will not change over time, as non-proxied objects are easier to debug and faster. This option is not available formake(Auto)Observable, see avoiding proxies.
There was a problem hiding this comment.
@liucan233 thanks for the PR! Mind following up on the review feedback posted earlier? Beyond that looking good to me so happy to merge.
|
Thank you for all pointing out my errors, and All errors have been corrected. Perhaps it will take you some time to check again. |
|
@liucan233 Hi, You don't need to force push your code (otherwise, we would need to trigger CI manually every time). If your PR is accepted, we will merge it using Squash and merge |
|
Thank you! |
Code change checklist
Added/updated unit tests
Updated
/docs. For new functionality, at leastAPI.mdshould be updatedVerified that there is no significant performance drop (
yarn mobx test:performance)